Add more strict verifications when user provides a platform #336
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Benchmark Test ResultsBenchmark results from the latest changes vs base branch |
Signed-off-by: Alex Goodman <[email protected]>
wagoodman
force-pushed
the
fix-platform-options
branch
from
a7b731e to
9c7e279
Compare
kzantow
reviewed
Jan 4, 2025
Signed-off-by: Alex Goodman <[email protected]>
kzantow
approved these changes
Jan 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Today we allow for users to specify a platform for images being resolved. The expectation is that if the fetched/resolved image does not match the requested platform then we should error out and not continue. Bugs were found in both how the Docker daemon provider (thus Podman as well since this is shared code) as well as the OCI registry provider. Though the bugs were slightly different, at a high level they were the same: a user could provide a platform to fetch and the provider in some cases would ignore this platform and fetch the image for a different platform.
For the Docker daemon provider when monitoring the pull event status JSONL stream the
Errorfield was not being considered, which is where this kind of error is raised up.For the OCI registry provider, passing the
remote.WithPlatformoption only sets the platform field on thev1.Descriptorobject, but does not have any effect on what is fetched in the case where there is no manifest list or index (there is a single architecture in the registry). In the case of manifest list or index the correct platform was being resolved. The change here was to explicitly pull the container config and validate against theosandarchitecturefields (which are required via the OCI spec).An additional step within the OCI registry provider is checking if the manifest is a list/index or a single manifest. If the user does NOT give a platform then the current default behavior for this provider is to set one based off of
linux/<GOARCH>. The intended behavior is to honor what the single-architecture manifest instead of overriding with a default value. This PR enforces this intended behavior by clearing the platform options after fetching the manifest and checking the MediaType for single vs multi arch support.The last change is to allow for providers to express errors that indicate that the image was fully resolved by the provider but there was a transient or fundamental error. This new error is intended to signal to the caller that no further providers in a set of providers should be attempted, and to raise up this single error. For instance, if we tried 7 providers with 7 "not found" errors and we reach the Docker provider and it raises up a platform mismatch error, we don't want to attempt the Registry provider since we expect a similar outcome. This fosters a fail-fast approach for cases we know the user should be paying attention to (and not get lost in a pile of errors from multiple providers).Instead a new specific error type has been added to allow the caller to more clearly reason about platform mismatch errors.